Allow Range through realm-server CORS for authenticated media requests - #5798

[Claude Code 🤖] Review lens: does the CORS surface actually let an authenticated, cross-origin, ranged media request through — the preflight approves the request headers the service worker sends, and the 206 exposes the headers a native player needs — and is every home of that contract updated, not just one.

Bottom line: no blocking issues. The change is correct and the mechanism reasoning in both the PR body and the code comments checks out against the middleware source. Two inline confirmations below document why it's right so the next editor doesn't undo it; both non-blocking.

What lands right (verified against @koa/cors@4.0.0 source, not just asserted):

• The "replace, not merge" claim is real. For an actual (non-OPTIONS) response, @koa/cors sets Access-Control-Expose-Headers from its exposeHeaders before await next() (node_modules/.pnpm/@koa+cors@4.0.0/.../index.js:95-105); the handler then runs, and setContextResponse (packages/realm-server/middleware/index.ts) copies createResponse's headers onto the Koa context with ctxt.set(...), overwriting it. So on any realm-handler response, createResponse's list is final — which is exactly the list the media 206 carries. Updating createResponse is therefore the load-bearing half; updating the @koa/cors exposeHeaders covers responses that never go through createResponse.
• The preflight list is final. For OPTIONS, @koa/cors sets Access-Control-Allow-Headers from the configured allowHeaders and returns 204 without invoking the handler (index.js:155-163), so nothing downstream can drop Range/If-Range.
• Twin check came back clean. The only other CORS header sites — proxy-forward.ts (SSE / proxyAsset for the auth-SW script), the host dev-CORS middleware, matrix-backend-authentication.ts — none serve realm file bytes, so none need the range headers. The realm server's cors() + createResponse are the two real homes, and both are updated.

Test coverage: the two tests pin the two winning lists — the preflight's Allow-Headers and the 206's Expose-Headers (which is createResponse's). Worth knowing that the @koa/cors exposeHeaders addition on server.ts is not exercised by either test, because every range response is produced by createResponse and so never surfaces the middleware's list; it's belt-and-suspenders for a hypothetical non-createResponse range response. Fine to keep, just uncovered.

Adjacent, out of scope (correctly deferred): end-to-end browser verification (service worker → preflight → authed 206s → native playback) is left to the FileDef-player follow-up, which is the right place for it — this PR's unit/integration level is the right altitude for the CORS contract itself.

[Claude Code 🤖] Review lens: the change itself is unchanged since the earlier pass — byte-for-byte — so this pass re-verified the CORS contract from scratch against the moved base (which now carries the byte-range file serving this depends on), and went after what the earlier pass didn't: whether the reason recorded in the comments matches the mechanism that actually consumes these headers, and whether the new tests fail when they should.

Bottom line: no blocking issues. The header lists are correct in both homes and the replace-not-merge reasoning in the description reproduces exactly. Four non-blocking comments: two confirmations that document why the code is right (including one asymmetry that looks like a bug and isn't), one comment-wording suggestion, and two test observations.

On the rebase. Comparing each tip against its own parent, the diff is identical to what was reviewed before — same three files, same 108/3. Only the base moved, onto a main where byte-range serving is merged. Re-checked the contract against that base rather than assuming it carried over: every 206 and the 416 in Realm's file-serving path is built with createResponse, so create-response.ts remains the list that reaches the wire on a media range response.

What lands right (verified, not inherited). I re-derived the middleware behavior with a standalone Koa app on @koa/cors@4.0.0 — cors() middleware, then a handler that builds a Response and copies it onto the context with the same per-header ctx.set() loop setContextResponse uses:

• Replace-not-merge is real, and it is per-header. @koa/cors sets Access-Control-Expose-Headers at index.js:95-96 and only reaches await next() at index.js:105/108, so the middleware's value is always in place before the handler runs. setContextResponse then iterates only the headers present on the handler's Response and ctxt.set()s each — so a response carrying its own expose list replaces the middleware's outright, and a response without one keeps it. Updating createResponse was mandatory; updating the middleware's list covers the responses that never touch a createResponse handler.
• The preflight list is final. For OPTIONS, @koa/cors returns 204 with Access-Control-Allow-Headers echoing the configured list verbatim and never invokes the handler — nothing downstream can drop Range/If-Range. Confirmed in the repro alongside Access-Control-Max-Age: 86400.
• Twin check came back clean, independently. The only other sites that set CORS headers are proxy-forward.ts (setupSSEHeaders, text/event-stream only), matrix-backend-authentication.ts (exposes Authorization alone), and the host dev-CORS middleware. None serves realm file bytes. There is no static-file middleware in the realm server that could emit a 206 outside createResponse. The --cors=…,Range,… list on the icon static server is a separate surface serving built icon assets and needs nothing from this change.
• No deploy-time lag from the 24 h preflight cache, which is a fair thing to wonder about given maxAge: 86400. Per the Fetch spec the CORS-preflight cache is keyed per header name, and a request carrying a name with no cache entry forces a fresh preflight — so a newly allowed Range can't be answered from a pre-existing entry. Spec-derived, not measured here.

CI. Realm-server tests are green (2,174 passing, 0 failing) including the two new ones. One host shard is red, and it emitted no junit at all — it died before test output rather than on an assertion, so it reads as infrastructure. Nothing in this diff can reach host tests: both changes are strictly additive CORS permissions, an allow list and an expose list, and neither can turn a request that previously succeeded into a failure. Worth a re-run rather than an investigation.

Recommendations, all non-blocking:

1. Reword the Content-Range/Accept-Ranges rationale in create-response.ts — the real consumer is the media element's own loading stack, not app JS, because the service worker returns the CORS-filtered Response to it via respondWith. Stronger reason, and the weak version is what invites a future trim. See the thread on create-response.ts.
2. Assert Access-Control-Allow-Origin in the 206 test — an expose list is inert without it, so today the test pins half its contract. Also assert the upload's 204. See the thread on the ranged-response test.
3. Fold the two CORS tests into range-request-test.ts, whose setup block is byte-identical and which already has uploadSample/binaryParser and no sample.png. See the file-level thread on cors-range-test.ts.
4. Nothing to change for the Location asymmetry between the two expose lists — it reads like the silent drop the comments warn about but is safe, and the thread on server.ts records why so it isn't "fixed" and the invariant isn't dismissed.

Adjacent, out of scope. packages/host/public/auth-service-worker.js still documents itself as being for <img> and CSS background-image, while the header comments added here correctly describe it as the path native <audio>/<video> depend on. Whoever next touches that file should widen its opening comment to match. Also agreed on deferring browser-level end-to-end verification of protected-realm playback to the player integration work — this PR's level is the right altitude for the CORS contract itself.

Generated by Claude Code